Skip to content

Allow installation of extra platform plugins #118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 26, 2022

Conversation

asl
Copy link
Contributor

@asl asl commented Jul 21, 2022

Resolves #88

@asl
Copy link
Contributor Author

asl commented Jul 21, 2022

This is essentially code from #89 but refactored to address review requests.

I checked that indeed it does deploy extra platform plugins and the final AppImage works

@asl
Copy link
Contributor Author

asl commented Jul 22, 2022

Tagging @TheAssassin

@bjorn
Copy link
Contributor

bjorn commented Jul 22, 2022

I'm just wondering, but why do we need EXTRA_PLATFORM_PLUGINS in addition to EXTRA_QT_PLUGINS? Are platform plugins not Qt plugins as well?

@asl
Copy link
Contributor Author

asl commented Jul 22, 2022

@bjorn Apparently, yes. EXTRA_QT_PLUGINS is a pre-defined hardcoded list:

static const std::vector<QtModule> Qt5Modules = {

Actually EXTRA_QT_PLUGINS name is a bit misleading, it should actually be EXTRA_QT_MODULES.

@asl
Copy link
Contributor Author

asl commented Jul 27, 2022

@TheAssassin will you be able to review this? Thanks!

@asl
Copy link
Contributor Author

asl commented Aug 23, 2022

@TheAssassin gently ping

@TheAssassin
Copy link
Member

Actually EXTRA_QT_PLUGINS name is a bit misleading, it should actually be EXTRA_QT_MODULES.

I think this was just taken over from linuxdeployqt. We should think about correcting this. Mind to open an issue?

@asl asl force-pushed the extra-platform-plugins branch from 0038984 to c20ae97 Compare August 26, 2022 11:40
@asl asl requested a review from TheAssassin August 26, 2022 11:41
@TheAssassin
Copy link
Member

Haven't tested this extensively, but the code looks good and on a first try it seems to do the trick. Thanks @asl!

@TheAssassin TheAssassin merged commit 76a163b into linuxdeploy:master Aug 26, 2022
@asl asl deleted the extra-platform-plugins branch August 26, 2022 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use of different platforms
3 participants